Skip to content

fix(lint): flow 规则能看进 try_catch / loop / parallel 嵌套 region(#4380) - #4388

Merged
os-zhuang merged 1 commit into
mainfrom
claude/flow-nested-region-walk
Jul 31, 2026
Merged

fix(lint): flow 规则能看进 try_catch / loop / parallel 嵌套 region(#4380)#4388
os-zhuang merged 1 commit into
mainfrom
claude/flow-nested-region-walk

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Closes #4380

问题

packages/lint 里每一条走 flow 节点的规则都手写了同一行:

const nodes = Array.isArray(flow.nodes) ? (flow.nodes as AnyRec[]) : [];

于是它们全都对同一件事失明。FlowRegionSchema 持有完整的 nodes: z.array(FlowNodeSchema),四个 config 槽位带 region:try_catch.config.try / .catchloop.config.bodyparallel.config.branches[].nodes,而且可以任意嵌套。节点一挪进去,检查就留在了外面。

同一组坏节点,顶层 vs try_catch 内部,实测:

规则 严重级 顶层 嵌套
flow-node-write-unknown-field error 1 0
flow-update-readonly-field error 1 0
approval-approver-* error/warning 1 0
flow-template-unknown-field(filter 位置) error 1 1,但降级成 warning

最后一行才是最麻烦的

validate-flow-template-paths 对节点整个 config 做递归 string-leaf 扫描,所以它碰巧还看得见嵌套节点里的 token——但 collectNodeLeaves 只在拿到的那个节点的顶层拆 filter 键。嵌套后位置信息丢失:

FLAT     error  	flow "f" node "get_record"	flows[0].nodes[1]
NESTED   warning	flow "f" node "try_catch"  	flows[0].nodes[1]

于是 #3810 那条「条件被抹掉会放宽查询、运行时直接拒绝执行该节点」的 gating 判断,变成一行黄色 advisory,还定位到外层 try_catch 而不是真正跑不了的 get_record

看得见 ≠ 判断对了,而且这比干脆漏掉更糟:一行黄色读起来像「已经检查过,只是建议性的」。

修法:一次遍历,不是五份

新增 flow-walk.ts——包里已有的 page-walk.ts 的 flow 侧对应物,理由也照抄它自己写下的那句:这个遍历微妙到「复制它已经造出过一条死规则」。

walkFlowNodes(flow, flowPath) 产出每个节点及其真实 config 路径(flows[0].nodes[1].config.catch.nodes[0])、用于诊断的 region 面包屑(try_catch "Guard" › catch)和深度。四条规则改走它。

Finding 现在落在真正出问题的那个节点上——在有多个 region 的流里,一个指向容器的路径没法据以行动。

重复计数的坑一次性解决,不留给每个调用方

容器节点也会被产出(它自己的 config 值得检查——loopcollectiontry_catchretry),但它的 config 物理上包含所有后代,所以递归扫 config 的规则会把每个嵌套 finding 报两遍。WalkedFlowNode.localConfig 是剥掉 region 槽位后的容器 config,递归扫描方用它。测试钉住:嵌套 token 只报一次,同时容器自己的 collection token 照报。

防漂移

REGION_SLOTS 写成数据,并对着 spec 自己的 region-bearing config schema 行为式推导(槽位 = 接受 {nodes: […]} 的键)做 pin,而不是复述——所以第五种构造会让测试失败,而不是变成第五个静默盲区。MAX_REGION_DEPTH 防止手写(未 parse)的 stack 把 lint 挂住。

验证

  • 端到端:嵌套现在与顶层逐条一致,包括恢复的 error 严重级;
  • app-showcase 里 showcase_resilient_synccatch 分支中有一个 update_record,此前从未被任何规则检查过。它字段是对的,所以校验依旧干净;故意把字段名改坏后,os validate 现在 exit 1 并报出:
flow "showcase_resilient_sync" › try_catch "Push with retry" › catch › node "Flag Sync Failure":
  update_record writes 'sync_statuss', but object 'showcase_task' declares no such field. …
  rule: flow-node-write-unknown-field  at flows[24].nodes[1].config.catch.nodes[0].config.fields.sync_statuss
  • 三个 example 全部 exit 0;@objectstack/lint 43 文件 / 689 测试全绿(新增 25 条),typecheck 干净。

🤖 Generated with Claude Code

…4380)

Every lint rule that inspects flow nodes had hand-written the same one-liner —
`const nodes = Array.isArray(flow.nodes) ? flow.nodes : []` — and every one was
therefore blind to the same thing. `FlowRegionSchema` holds a full
`nodes: z.array(FlowNodeSchema)`, and four config slots carry one:
`try_catch.config.try` / `.catch`, `loop.config.body`, and
`parallel.config.branches[].nodes`. Regions nest arbitrarily. Move a node into
any of them and the checking stayed behind.

Measured, same bad nodes flat vs inside a try_catch:

  flow-node-write-unknown-field (error)  1 → 0
  flow-update-readonly-field    (error)  1 → 0
  approval-approver-*                    1 → 0
  flow-template-unknown-field   (error)  1 → 1, but as a WARNING

The last one is the one a reader would not predict. This rule scans a node's
whole config for string leaves, so it still SAW tokens inside a region — but its
`filter`-position split only looks at the top level of the node it was handed.
A nested filter token lost its position, so the #3810 finding ("this node cannot
run — an erased condition WIDENS the query") degraded to an advisory warning,
reported against the wrapping try_catch instead of the get_record that is
broken. Being visible is not the same as being judged correctly, and that is
worse than a clean miss: a yellow line reads as "checked, merely advisory".

One shared walk, not five. `flow-walk.ts` is the flow-side counterpart of the
existing `page-walk.ts`, here for the same stated reason — getting the traversal
right is subtle enough that duplicating it has already produced dead rules.
`walkFlowNodes` yields every node with its real config path
(`flows[0].nodes[1].config.catch.nodes[0]`), a region breadcrumb for diagnostics
(`try_catch "Guard" › catch`), and depth. Four rules route through it. Findings
now land on the node that is actually wrong — a path pointing at the container
is not actionable in a flow with several regions.

The double-count trap is handled once, not left to each caller. A container is
walked too (its own config is worth checking — a loop's `collection`, a
try_catch's `retry`), but its config physically contains every descendant, so a
recursive scanner would report each nested finding twice. `localConfig` is the
container's config with region slots removed; a test pins that a nested token is
reported once while the container's own token still is.

`REGION_SLOTS` is data, pinned against the spec's region-bearing config schemas
by behavioural derivation rather than restatement, so a fifth construct fails
that test instead of becoming a fifth blind spot. `MAX_REGION_DEPTH` keeps a
hand-authored (pre-parse) stack from hanging a lint.

Verified end to end: nested now matches flat on every rule, including the
restored error severity. app-showcase ships an update_record inside a `catch`
branch that had never been checked by anything; it is correct, so validation
stays clean, and breaking its field name on purpose now fails `os validate` at
`flows[24].nodes[1].config.catch.nodes[0].config.fields.sync_statuss`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Jul 31, 2026 10:25am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/l labels Jul 31, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/lint.

2 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/automation/hook-bodies.mdx (via @objectstack/lint)
  • content/docs/permissions/authorization.mdx (via @objectstack/lint)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@os-zhuang
os-zhuang merged commit af5b96b into main Jul 31, 2026
17 checks passed
@os-zhuang
os-zhuang deleted the claude/flow-nested-region-walk branch July 31, 2026 10:39
os-zhuang added a commit that referenced this pull request Jul 31, 2026
…4401) (#4408)

A region is a sub-graph inside `FlowNodeSchema.config`, an open `z.record`.
Nothing in the type system says which key on which node type holds one, so every
pass that needs to reach a region node has to be told — and within one week three
of them were told separately, by two changes that were each correct on their own
(#4381 and #4388):

  mapFlowNodes (ADR-0087 conversions)                spec   FLOW_REGION_SLOTS
  validateControlFlow / normalizeControlFlowRegions
    / collectFlowGraphs                              spec   regionSlotsOf
  walkFlowNodes (lint flow rules)                    lint   REGION_SLOTS

Each pinned its own copy with its own reconciliation test. So every copy was
protected from drifting away from the schemas, and nothing would have failed if
the copies drifted from each other — while adding a fourth construct meant
editing three places, and missing one reproduces exactly the silent blind spot
#4347 and #4380 were both filed about.

- New `@objectstack/spec/automation` export `FLOW_REGION_SLOTS` (+ the
  `FLOW_REGION_SLOTS_BY_TYPE` / `FLOW_REGION_CONFIG_KEYS` views) is the only
  statement of the fact. Import-free module, so `spec/conversions/walk.ts` can
  read it and stay the pure shape walker it was written as; mapping a slot onto
  the Zod schema its value parses as stays in `control-flow.zod.ts`.
- The three reconciliation tests collapse into `region-slots.test.ts`, keeping
  the strongest of them (#4388's): it derives each construct's region keys
  BEHAVIOURALLY, by asking the config schema what it accepts in a region shape,
  rather than reading names off `.shape`. It also probes every other exported
  `*ConfigSchema`, so a new region-bearing construct cannot be added without
  either declaring its slots or failing here.

The three walks are deliberately left separate: different inputs (parsed vs raw
authored records), different units (a graph, a node, a copy-on-write rewritten
tree), and the lint one formats human diagnostic trails from node labels —
consumer logic, not protocol (Prime Directive #2).

No behaviour change: every existing test passes unchanged.


Claude-Session: https://claude.ai/code/session_01HSBbKMdDgHjGpQdrvQUQXj

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

1 participant